Skip to content

Fix #7138: Suspend callers of macros compiled in the same run #7324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 31 commits into from
Oct 28, 2019
Merged

Fix #7138: Suspend callers of macros compiled in the same run #7324

merged 31 commits into from
Oct 28, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Sep 26, 2019 •
edited
Loading

Allows macros to be defined and called in the same project. The idea is to suspend
the callers of a macro (and their upstream dependencies), compile the sourcefile(s)
containing the macro(s), and then compile the suspended files in a second run.

The scheme detects and flags as errors calls to macros in the same file and mutual
dependencies between different files which would not work with suspensions.

It needs to be complemented by another mechanism that checks that class files loaded
by a macro class loader do not have associated source files that are currently compiled.

@odersky

This comment has been minimized.

Sign in to view
@nicolasstucki

This comment has been minimized.

Sign in to view
@odersky

This comment has been minimized.

Sign in to view
@nicolasstucki

This comment has been minimized.

Sign in to view
@liufengyun

This comment has been minimized.

Sign in to view
@odersky

This comment has been minimized.

Sign in to view
@nicolasstucki

This comment has been minimized.

Sign in to view
@nicolasstucki
Copy link
Contributor

I tried converting all pos-macros and run-macros be compiled in a single step and use suspension. They all passed the tests. I will not add them to avoid duplication in tests.

nicolasstucki
nicolasstucki reviewed Sep 28, 2019
View reviewed changes
nicolasstucki
nicolasstucki reviewed Sep 30, 2019
View reviewed changes
@nicolasstucki nicolasstucki force-pushed the add-suspend branch 2 times, most recently from e436d28 to 4317e7c Compare October 2, 2019 14:28
@nicolasstucki nicolasstucki requested a review from smarter October 3, 2019 09:15
@nicolasstucki nicolasstucki mentioned this pull request Oct 3, 2019
Closed
@nicolasstucki nicolasstucki marked this pull request as ready for review October 3, 2019 11:20
@nicolasstucki nicolasstucki changed the title Suspend callers of macros compiled in the same run Fix #7138: Suspend callers of macros compiled in the same run Oct 4, 2019
@nicolasstucki nicolasstucki requested review from liufengyun and removed request for smarter October 4, 2019 15:54
@nicolasstucki nicolasstucki assigned liufengyun and unassigned smarter Oct 4, 2019
@nicolasstucki
Copy link
Contributor

nicolasstucki commented Oct 4, 2019 •
edited
Loading

@liufengyun the commits from @odersky LGTM but you should have a second look.

liufengyun
liufengyun approved these changes Oct 6, 2019
View reviewed changes
Copy link
Contributor

@liufengyun liufengyun left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

While the utility enabled by the PR is doubtless, this PR makes it easier to write code that has subtle dependencies than before. The current error messages may not be helpful enough to resolve cyclic compilation errors, which may hurt usability.

What about putting the feature under a compiler flag?

@nicolasstucki
Copy link
Contributor

@liufengyun the incremental compilation example actually works (or fails as expected). The incremental compiler trigger recompilation of macros when b is recompiled which makes it fail the same way it would from a clean compilation. I added it as a regression test.

odersky and others added 26 commits October 28, 2019 10:02
@odersky @liufengyun
Add debug output to track down problem compiling ScalaTest
38784ab
@odersky @liufengyun
Add -Xprint-suspension option to print info about suspensions
58d7f5e
@odersky @liufengyun
Compile scalatest with -Xprint-suspension
ce557d6
@odersky @liufengyun
Don't count vals as macro dependencies
b2d2796
@nicolasstucki @liufengyun
Move tests to pos-macros
440e295
@nicolasstucki @liufengyun
Fix error message
463aa7b
@nicolasstucki @liufengyun
Fix reflect-inline test
cd0a9ce
@nicolasstucki @liufengyun
Revert "Don't count vals as macro dependencies"
9e387f8 
This reverts commit 8cea720.
@nicolasstucki @liufengyun
Add minimization of previous scalatest stripMargin issue
7ebdcf0
@nicolasstucki @liufengyun
Suspend macros when class not found during macro expansion
c7b9326
@nicolasstucki @liufengyun
Improve error message
6d5ef8a
@odersky @liufengyun
Don't suspend if errors were reported.
1babee2
@nicolasstucki @liufengyun
Identify classes defined in current run
ff9c07c
@nicolasstucki @liufengyun
Remove outdated comment
3a8c56f
@nicolasstucki @liufengyun
Handle suspended units in InteractiveDriver
56f3b2c
@nicolasstucki @liufengyun
Test for legitimate NoClassDefFoundError in Splicer
e80aefd
@nicolasstucki @liufengyun
Add regression tests
14bf2c6
@nicolasstucki @liufengyun
Add output as macro classpath for suspended compilation
e1c8fdf
@nicolasstucki @liufengyun
Add error when suspension has jar output
9134d3f 
To avoid a compiler crash.
@nicolasstucki @liufengyun
Add supspension to macro docs
a4bdb4f
@nicolasstucki @liufengyun
Test incremental compilation with suspension
ca57ff9
@nicolasstucki @liufengyun
Typos and specify run vs compile time
4c353c6
@nicolasstucki @liufengyun
Add test on incremental and suspended compilation
0c33e64
@liufengyun
Fix check file
5d863fb
@liufengyun
Fix trailing space in error message
8457680
@liufengyun
Fix CI: remove trailing spaces in check files
4f59892
@liufengyun
Copy link
Contributor

rebased to fix conflicts

@anatoliykmetyuk anatoliykmetyuk merged commit 9193c44 into scala:master Oct 28, 2019
@anatoliykmetyuk anatoliykmetyuk deleted the add-suspend branch October 28, 2019 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liufengyun liufengyun liufengyun approved these changes

@nicolasstucki nicolasstucki nicolasstucki approved these changes

Assignees

@liufengyun liufengyun

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@odersky @nicolasstucki @liufengyun @smarter @anatoliykmetyuk